esm: avoid super-linear data URL MIME regex#61951
esm: avoid super-linear data URL MIME regex#61951skdas20 wants to merge 2 commits intonodejs:mainfrom
Conversation
|
Review requested:
|
nodejs-github-bot
added
esm
jsumners-nr
left a comment
jsumners-nr
left a comment
There was a problem hiding this comment.
Where are tests to assert the undesired behavior is resolved by the changes?
|
Added regression coverage in The test exercises |
| // Flags: --expose-internals | ||
| 'use strict'; | ||
|
|
||
| const common = require('../common'); | ||
| const assert = require('assert'); | ||
| const { defaultGetFormat } = require('internal/modules/esm/get_format'); | ||
|
|
||
| // Regression test for #61904. This malformed data URL path is crafted to | ||
| // stress the MIME regex without triggering URL parsing failures. | ||
| const longPath = `data:a/${'a'.repeat(2 ** 17)}B`; | ||
| const start = process.hrtime.bigint(); | ||
| const format = defaultGetFormat(new URL(longPath), { parentURL: undefined }); | ||
| const elapsedMs = Number(process.hrtime.bigint() - start) / 1e6; | ||
|
|
||
| assert.strictEqual(format, null); | ||
| assert.ok( | ||
| elapsedMs < common.platformTimeout(1000), | ||
| `Expected format detection to complete quickly, took ${elapsedMs}ms`, | ||
| ); |
There was a problem hiding this comment.
This is going to be inherently flaky, could we instead write a benchmark?
|
CI has not started yet on this PR (currently no check runs, and Could a collaborator please add the |
|
There are no official approvals on this PR, so CI cannot be started (it will refuse). |
5 participants
Fixes: #61904
This updates ESM data URL MIME extraction regexes to remove overlapping quantifiers that allow super-linear backtracking.
(?:[^,]*?)(;base64)?,with(?:;[^,]*)?,in:lib/internal/modules/esm/get_format.jslib/internal/modules/esm/load.jsBehavior is preserved for existing valid/invalid
data:URL shapes while avoiding pathological backtracking on crafted inputs.